Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

max pool Layer with mask #4891

Merged
merged 19 commits into from
Nov 14, 2017
Merged

Conversation

NHZlX
Copy link
Contributor

@NHZlX NHZlX commented Oct 18, 2017

fix #4889

python usage:

tmp = paddle.layer.img_pool(
                         input=tmp,
                         pool_size=7,
                         stride=1,
                         pool_type=paddle.pooling.MaxWithMask())

@NHZlX NHZlX requested a review from hedaoyuan October 18, 2017 11:22
const int tgtStride) {
const int tgtStride,
real* maskData,
bool withMask) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not need withMask, we can check whether maskData a NULL pointer.

* @param[out] tgtData output data.
* @param[in] tgtStride stride between output data samples.
* @param[out] maskData the location indices of select max data
* @param[in] withMask set true if output maskData
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove line 77 and 78.

maxval = inputData[h * width + w];
max_index = h * width + w;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maxval = inputData[max_index];

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

* @param[out] maskData the location indices of select max data
* @param[in] withMask set true if output maskData
*/
extern void hl_maxpool_forward(const int frameCnt,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no need to add this interface. Only need to add a maskData parameter to the original interface.

@@ -37,6 +37,8 @@ class PoolLayer : public Layer {
int confPaddingY_;

std::string poolType_;
bool with_mask_;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not need with_mask_.

false);
}

void GpuMatrix::maxPoolForward(Matrix& inputMat,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, do not need to add a new interface.

@@ -69,6 +69,17 @@ class Projection {
forward();
}

void forward(const Argument* in,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not change this file.
We can add a PoolMaxWithMaskLayer, and create this layer in the PoolLayer::create when pool == "max-pool-with-mask" .

Copy link
Contributor

@hedaoyuan hedaoyuan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not modify the implementation of the original Layer. The current Operator refactoring is based on the Layer, modify the original Layer implementation may be confused on the reconstruction. You can add a PoolMaxWithMaskLayer to avoid modifying the original layer.

for (int w = wstart; w < wend; ++w) {
outData[ph * outputW + pw] = std::max(
outData[ph * outputW + pw], inputData[h * imgSizeW + w]);
if (maskMatP == NULL) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the if (maskMatP == NULL) can be inside the for loop? And use maskData == NULL instead of maskMatP == NULL.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

}
}
}
}
// compute offset
inputData += inLength;
outData += outLength;

if (maskMatP != NULL) maskData += outLength;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use maskData != NULL instead of maskMatP != NULL.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

CudnnMaxPooling], \
"only (Cudnn)AvgPooling, (Cudnn)MaxPooling are supported"
"only (Cudnn)AvgPooling, (Cudnn)MaxPooling MaxWithMaskPooling are supported"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add , before MaxWithMaskPooling

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

sizeY_,
confPaddingY_,
strideY_,
/* caffeMode */ false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it can be modified into caffeMode = true? The other Layer (Pooling, Conv) default is true.

Copy link
Contributor Author

@NHZlX NHZlX Nov 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The caffeMode in poolProjection.cpp is false, i referred there. https://github.com/PaddlePaddle/Paddle/blob/develop/paddle/gserver/layers/PoolProjection.cpp#L51

maskMat->setData(maskData);
doOneMaxPoolingWithMaskOutputTest(
inputMat, "max-pool-with-mask", useGpu, maskMat);
/*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove line 108-118

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forgot to delete the annotation symbol, it supports the gpu test

pool->set_stride(sw);
pool->set_stride_y(sh);

int ow = outputSize(pool->img_size(), kw, pw, sw, /* caffeMode */ false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you change the value of caffeMode in the Layer, this also needs to be modified.

@NHZlX NHZlX merged commit 3e6f768 into PaddlePaddle:develop Nov 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

need an 2D max pooling layer with mask output in paddle
2 participants